Skip to content

iox-#1755 Use upper camel case for LogLevel enum tags #2405

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
elBoberido merged 4 commits into from
Jan 8, 2025
Merged

iox-#1755 Use upper camel case for LogLevel enum tags #2405

elBoberido merged 4 commits into from
Jan 8, 2025

Conversation

@elBoberido
Copy link
Member

@elBoberido elBoberido commented Jan 8, 2025 •
edited
Loading

Notes for Reviewer

The ERROR log level causes issues on Windows since there is an ERROR define on the Windows platform. To prevent future issues, all LogLevel enum tags follow now the upper camel case naming convention.

The main issue is that upper snake case is usually used for defines and macros and with the short names, there is quite a high chance that those names are already used somewhere else. Using upper camel case should reduce the probability of name clashes.

It was also required to update the clang-format version.

Pre-Review Checklist for the PR Author

  1. Code follows the coding style of CONTRIBUTING.md
  2. Tests follow the best practice for testing
  3. Changelog updated in the unreleased section including API breaking changes
  4. Branch follows the naming format (iox-123-this-is-a-branch)
  5. Commits messages are according to this guideline
  6. Update the PR title
    • Follow the same conventions as for commit messages
    • Link to the relevant issue
  7. Relevant issues are linked
  8. Add sensible notes for the reviewer
  9. All checks have passed (except task-list-completed)
  10. Assign PR to reviewer

Checklist for the PR Reviewer

  • Consider a second reviewer for complex new features or larger refactorings
  • Commits are properly organized and messages are according to the guideline
  • Code according to our coding style and naming conventions
  • Unit tests have been written for new behavior
  • Public API changes are documented via doxygen
  • Copyright owner are updated in the changed files
  • All touched (C/C++) source code files from iceoryx_hoofs have been added to ./clang-tidy-diff-scans.txt
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

References

@elBoberido elBoberido requested a review from elfenpiff January 8, 2025 00:02
@elBoberido elBoberido self-assigned this Jan 8, 2025
@elBoberido elBoberido force-pushed the iox-1755-make-log-enum-tags-upper-camel-case branch 3 times, most recently from 56d928a to 2221186 Compare January 8, 2025 00:38
elfenpiff
elfenpiff previously approved these changes Jan 8, 2025
View reviewed changes
@elBoberido elBoberido force-pushed the iox-1755-make-log-enum-tags-upper-camel-case branch from 2221186 to 292dbe1 Compare January 8, 2025 00:50
@elBoberido elBoberido force-pushed the iox-1755-make-log-enum-tags-upper-camel-case branch from 7c11f82 to 4fb2c8d Compare January 8, 2025 00:58
@elBoberido elBoberido requested a review from elfenpiff January 8, 2025 00:58
@codecov
Copy link

codecov bot commented Jan 8, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 35.78501% with 454 lines in your changes missing coverage. Please review.

Project coverage is 78.69%. Comparing base (f6e2a98) to head (4fb2c8d).
Report is 5 commits behind head on main.

Files with missing lines Patch % Lines
iceoryx_hoofs/posix/filesystem/source/file.cpp 8.62% 53 Missing ⚠️
iceoryx_posh/source/runtime/posh_runtime_impl.cpp 48.27% 30 Missing ⚠️
iceoryx_hoofs/posix/sync/source/mutex.cpp 15.15% 28 Missing ⚠️
iceoryx_posh/source/roudi/roudi.cpp 21.21% 26 Missing ⚠️
...ceoryx_hoofs/posix/filesystem/source/file_lock.cpp 7.40% 25 Missing ⚠️
...oryx_hoofs/posix/ipc/source/unix_domain_socket.cpp 0.00% 25 Missing ⚠️
iceoryx_posh/source/roudi/port_manager.cpp 40.62% 19 Missing ⚠️
iceoryx_posh/source/roudi/process_manager.cpp 51.35% 18 Missing ⚠️
.../posix/design/source/file_management_interface.cpp 0.00% 15 Missing ⚠️
...ceoryx_hoofs/posix/ipc/source/posix_memory_map.cpp 0.00% 15 Missing ⚠️
... and 60 more
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2405   +/-   ##
=======================================
  Coverage   78.69%   78.69%           
=======================================
  Files         440      440           
  Lines       16981    16981           
  Branches     2361     2361           
=======================================
  Hits        13364    13364           
  Misses       2736     2736           
  Partials      881      881
Flag Coverage Δ
unittests 78.49% <35.78%> (ø)
unittests_timing 15.35% <0.28%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...ceoryx_binding_c/source/c2cpp_enum_translation.cpp 100.00% <100.00%> (ø)
iceoryx_binding_c/source/c_publisher.cpp 92.50% <100.00%> (ø)
iceoryx_binding_c/source/c_subscriber.cpp 92.94% <100.00%> (ø)
iceoryx_binding_c/source/c_user_trigger.cpp 94.44% <100.00%> (ø)
iceoryx_hoofs/concurrent/sync/source/spin_lock.cpp 82.00% <100.00%> (ø)
...yx_hoofs/concurrent/sync/source/spin_semaphore.cpp 92.30% <100.00%> (ø)
iceoryx_hoofs/filesystem/source/file_reader.cpp 94.73% <100.00%> (ø)
iceoryx_hoofs/filesystem/source/filesystem.cpp 77.88% <100.00%> (ø)
iceoryx_hoofs/memory/source/bump_allocator.cpp 100.00% <100.00%> (ø)
...oryx_hoofs/posix/design/include/iox/posix_call.hpp 100.00% <ø> (ø)
... and 92 more

@elBoberido elBoberido merged commit 8d4afc5 into eclipse-iceoryx:main Jan 8, 2025
23 checks passed
@elBoberido elBoberido deleted the iox-1755-make-log-enum-tags-upper-camel-case branch January 8, 2025 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@elfenpiff elfenpiff elfenpiff approved these changes

Assignees

@elBoberido elBoberido

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@elBoberido @elfenpiff